Skip to content

test(Rosenbrock): update resize_tests for new cache fields (k₁/k₂/k₃ → ks, f₁ → dense)#3313

Closed
singhharsh1708 wants to merge 2 commits intoSciML:masterfrom
singhharsh1708:fix-rosenbrock-resize-tests
Closed

test(Rosenbrock): update resize_tests for new cache fields (k₁/k₂/k₃ → ks, f₁ → dense)#3313
singhharsh1708 wants to merge 2 commits intoSciML:masterfrom
singhharsh1708:fix-rosenbrock-resize-tests

Conversation

@singhharsh1708
Copy link
Copy Markdown
Contributor

@singhharsh1708 singhharsh1708 commented Apr 1, 2026

Summary

Updates resize_tests.jl to reflect the new RosenbrockCache structure introduced in the RodasTableau refactor.

The previous tests accessed fields specific to Rosenbrock23Cache / Rosenbrock32Cache:

  • k₁, k₂, k₃
  • f₁

These have been replaced in the unified RosenbrockCache with:

  • ks (vector of stage values)
  • dense (interpolation coefficients)

Changes

  • Replaced:

    • k₁, k₂, k₃ks
    • f₁dense
  • Updated assertions to validate vectorized structure:

    • all(length.(cache.ks) .== N)
    • all(length.(cache.dense) .== N)

Rationale

The cache structure was unified as part of the RodasTableau migration, so tests need to align with the new representation.

No changes to solver behavior — this only updates tests to match the new internal structure.
Test for #3273

Checklist

  • Appropriate tests were updated
  • No public API changes
  • Follows SciML style guidelines
  • Documentation unaffected (internal change)

@test length(i.cache.k₁) == 5
@test length(i.cache.k₂) == 5
@test length(i.cache.k₃) == 5
@test all(length.(i.cache.ks) .== 5)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also test that there is the right number of ks.

@ChrisRackauckas
Copy link
Copy Markdown
Member

These have been replaced in the unified RosenbrockCache with:

That's only with your PR though? So this fails without your PR. Is there a reason this isn't in the PR branch?

@singhharsh1708
Copy link
Copy Markdown
Contributor Author

These have been replaced in the unified RosenbrockCache with:

That's only with your PR though? So this fails without your PR. Is there a reason this isn't in the PR branch?

image I’ve kept the Rosenbrock23/32 → RodasTableau migration PR focused on code changes only and removed all test updates from it as suggested. The test changes are now in a separate PR since they depend on the new RosenbrockCache structure (k₁/k₂/k₃ → ks, f₁ → dense). This PR is intended to be merged after the migration PR.

@ChrisRackauckas
Copy link
Copy Markdown
Member

I’ve kept the Rosenbrock23/32 → RodasTableau migration PR focused on code changes only and removed all test updates from it as suggested

No, unrelated test changes should be a separate PR. This is a test that must change because of your changes. Your changes change it from using k1, k2, k3 to a ks vector, so the update should be together. This cannot merge without that change because it would fail as it's simply not correct to change to a ks vector without moving the source code.

@singhharsh1708 singhharsh1708 deleted the fix-rosenbrock-resize-tests branch April 9, 2026 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants